Replace flake8 and isort with ruff #6128
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6128 +/- ##
==========================================
- Coverage 82.42% 82.41% -0.01%
==========================================
Files 350 350
Lines 21457 21423 -34
Branches 839 839
==========================================
- Hits 17685 17656 -29
+ Misses 3474 3469 -5
Partials 298 298
... and 4 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
dafeda
added
maintenance
dafeda
force-pushed
the
ruff
branch
2 times, most recently
from
26b6c17
to
a4968dd
Compare
|
I would like not to mix the shift from isort+flake8 to ruff with the actual fixing of the issues. Can we have a PR that only converts the style tooling (adding more exceptions if needed), and then fixing and removing exceptions in future PRs |
Not sure I agree since this makes it clear that the changes were done to please the linter, but I'll see what I can do. |
| @@ -167,8 +167,8 @@ def valid_num_iterations(user_input: str) -> str: | |||
| def attemp_int_conversion(val: str) -> int: | |||
| try: | |||
| return int(val) | |||
| except ValueError: | |||
| raise ArgumentTypeError(f"{val} is not a valid integer") | |||
| except ValueError as e: | |||
There was a problem hiding this comment.
This looks like an automated fix. Is it given that we always want to reraise exceptions and keep all the details (it probably often is). When seen by users, more details can make them blind.
If this is a new requirement imposed by ruff, it would like to have a global exception for it in ruff-configuration, and then we can think about solving it in a future PR.
There was a problem hiding this comment.
Not really automated, I just did not see any reason to hide the context and yes, it makes ruff happy.
Also, I'm not really sure what the difference would be between the two in the specific function.
I would guess that they do the same thing.
| @@ -266,7 +266,7 @@ def get_documentation_for_jobs(self) -> Dict[str, Any]: | |||
| self.hook.installable_jobs(), include_plugin_data=True | |||
| ).items() | |||
| } | |||
| for k in job_docs.keys(): | |||
| for k in job_docs: | |||
There was a problem hiding this comment.
Do you know what k is? Not introducing it, I know, but if you know, perhaps rename it?
There was a problem hiding this comment.
I don't know what it is unfortunately.
| @@ -35,9 +35,7 @@ def test_using_qt_model_tester(qtmodeltester, full_snapshot): | |||
| model.setSourceModel(source_model) | |||
|
|
|||
| reporting_mode = qt_api.QtTest.QAbstractItemModelTester.FailureReportingMode.Warning | |||
| tester = qt_api.QtTest.QAbstractItemModelTester( # noqa, prevent GC | |||
There was a problem hiding this comment.
Perhaps relevant to keep the prevent GC part of the comment?
| @@ -56,9 +54,7 @@ def test_changes(full_snapshot): | |||
| model.setSourceModel(source_model) | |||
|
|
|||
| reporting_mode = qt_api.QtTest.QAbstractItemModelTester.FailureReportingMode.Warning | |||
| tester = qt_api.QtTest.QAbstractItemModelTester( # noqa, prevent GC | |||
Flake8 does not support pyproject.toml out-of-the box and ruff is faster.
Pre review checklist
Ground Rules),
and changes to existing code have good test coverage.
Pre merge checklist